Skip to content

Conversation

@NillRudd
Copy link
Contributor

@NillRudd NillRudd commented Oct 5, 2025

Fixes #833

Summary

  • Accept node IDs in decimal, !hex, and 0xhex; normalize to int.
  • Deduplicate lora.ignore_incoming on write.
  • Support clearing via config.lora.ignore_incoming: [] reliably.
  • Prevent crash: 'bytes' object cannot be interpreted as an integer.
  • Removed some duplicate code.

Why

CLI operations around ignore list were brittle:

  • Occasional crash when adding IDs.
  • Duplicate entries possible.
  • Inconsistent ID formats accepted.
  • Clearing via YAML sometimes confusing.

Behavior

  • --set lora.ignore_incoming <id> accepts dec/!hex/0xhex and stores one normalized entry.
  • --get lora.ignore_incoming reflects deduped list.
  • YAML config.lora.ignore_incoming: [] clears list.

Notes

Firmware still shows packets in logs; enforcement is firmware-level and out of scope here.
No breaking changes—CLI becomes more permissive and robust.

@NillRudd
Copy link
Contributor Author

NillRudd commented Nov 4, 2025

Is it possible to get a review on this PR or get it merged? Would be deeply appreciated! @ianmcorvidae

@ianmcorvidae
Copy link
Contributor

Hi, I've been short on time lately but giving it a look now. It's worth noting that lora.ignore_incoming is mostly if not completely depreated -- the --set-ignored-node and --remove-ignored-node arguments that modify the nodedb are much preferred. But we may as well clean up the old version if it can be, so I'll give it a look.

Copy link
Contributor

@ianmcorvidae ianmcorvidae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, one thing I'd like to see moved elsewhere. I'll keep an eye on this and probably merge it soon if you don't get back to it.

Comment on lines 55 to 68
def _to_node_num(self, nodeId: Union[int, str]) -> int:
"""Normalize node id from int | '!hex' | '0xhex' | 'decimal' to int."""
if isinstance(nodeId, int):
return nodeId
s = str(nodeId).strip()
if s.startswith("!"):
s = s[1:]
if s.lower().startswith("0x"):
return int(s, 16)
try:
return int(s, 10)
except ValueError:
return int(s, 16)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this structure, but I think this function belongs in meshtastic.util, perhaps something like to_node_num there. (side note that most of the functions there are camelcase rather than underscores, but I think we may as well follow pep8 when we can).

Doesn't need to block merging, but if you're able to move this there that'd be appreciated!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has now been moved to util! Seems to be a bit of mixture between camelcase and underscores but i kept to_node_num.

@NillRudd NillRudd requested a review from ianmcorvidae November 6, 2025 18:10
Copy link
Contributor

@ianmcorvidae ianmcorvidae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@ianmcorvidae
Copy link
Contributor

heh, I think that failure is due to another PR. I'm gonna merge it and fix locally if needed

@ianmcorvidae ianmcorvidae merged commit 87682c1 into meshtastic:master Nov 6, 2025
5 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: lora.ignore_incoming crash (bytes→int), duplicates, and ID format inconsistency

2 participants